Skip to content

Feature/4202 flags clean#2556

Closed
amdomanska wants to merge 13 commits intodevelopfrom
feature/4202_flags_clean
Closed

Feature/4202 flags clean#2556
amdomanska wants to merge 13 commits intodevelopfrom
feature/4202_flags_clean

Conversation

@amdomanska
Copy link
Copy Markdown
Contributor

@amdomanska amdomanska commented Feb 16, 2026


Make Flag's Assignee and Deadline fields compulsory

  • Make assignee and deadline fields required: These fields must be filled if there is any value in the note field, using the RequiredIf validator.
  • Skip BigEnd validation for empty deadlines: The BigEnd validator is disabled when the deadline field is empty.
    *Validate only active fields: Validation runs only on non-disabled fields to avoid unnecessary errors.
  • Handle resolved flags: When a flag is resolved, all fields are disabled, preventing validation on already resolved flags.

This PR...

  • has scripts to run
  • has migrations to run
  • adds new infrastructure
  • changes the CI pipeline
  • affects the public site
  • affects the editorial area
  • affects the publisher area
  • affects the monitoring

Developer Checklist

Developers should review and confirm each of these items before requesting review

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Reviewer Checklist

Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section should be duplicated for each reviewer

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Testing

Feature testing:

Main feature test: https://doaj.github.io/doaj-docs/feature/4202_flags/testbook/index.html#flagged_journals/flagged_journals

Regression testing:

All other flagged journals tests: https://doaj.github.io/doaj-docs/feature/4203_flags_notifications/testbook/index.html#flagged_journals/flagged_journals

Deployment

No deployment considerations

@amdomanska amdomanska mentioned this pull request Feb 16, 2026
22 tasks
… front end validation doesn't run. The back end validation is correct too, but an invalid resolved flag still fails validation in the back end, because it has to be resolved, so a further fix is required, tbd
@richard-jones richard-jones self-requested a review February 24, 2026 11:39
Copy link
Copy Markdown
Contributor

@richard-jones richard-jones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed this and made some changes to normalise the validation between the front and back ends, and to remove the magic string approach used for validation initially.

This looks good to me now.

Copy link
Copy Markdown
Contributor

@brendan-oconnell brendan-oconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Aga,

I was testing the testbook to prepare to send this to the Managing Editors, and would like you to make some changes to the testbook so it's more clear.

I was using this testbook: https://doaj.github.io/doaj-docs/feature/4202_flags/testbook/index.html#flagged_journals/flagged_journals

I completed only section 1. Admin - Add, Edit, Resolve

Could you review the other sections to make sure the instructions are clear?

I will comment separately on the Issue with some bugs I noticed with the feature itself.

Thank you!!

@amdomanska
Copy link
Copy Markdown
Contributor Author

amdomanska commented Mar 26, 2026

Updated with the latest develop: 2026-03-26

@Steven-Eardley
Copy link
Copy Markdown
Contributor

Superseded by #2569 - this has been fully merged into feature/4202-4203_flags_combined

@Steven-Eardley Steven-Eardley deleted the feature/4202_flags_clean branch March 31, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants